-
-
Notifications
You must be signed in to change notification settings - Fork 527
Separate outputs for CommonJS and ESM when building openapi-typescrip… #1479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 935d4e7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix! Thank you!
Hm the failing tests might be from a race condition while building As a workaround, in the root, I’d recommend doing something like: "scripts": {
"build": "run-p -s build:*",
"build:openapi-typescript": "cd packages/openapi-typescript && pnpm run build",
- "build:openapi-typescript-helpers": "cd packages/openapi-typescript-helpers && pnpm run build",
- "build:openapi-fetch": "cd packages/openapi-fetch && pnpm run build",
+ "build:openapi-fetch": "cd packages/openapi-typescript-helpers && pnpm run build && cd ../openapi-fetch && pnpm run build" The TS helpers build is basically instant/trivial. And this keeps openapi-fetch and openapi-typescript built in parallel, which while both are quick, still makes a noticeable difference in CI runs |
Thanks, I've applied your fix. |
Hm something else seems to be going on. Will take a closer look later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working for me locally, and seems to just be a weird CI issue. I’ll merge this, then poke at it separately.
Oh I figured it out—it was a CI thing. It was just in the But really it’s just a flimsy setup; I’ll make some changes that make that better (also we shouldn’t have a |
…t-helpers
Changes
Fixes #1478
Creates separate output directories for CommonJS and ESM for the output of
openapi-typescript-helpers
.How to Review
pnpm build
packages/openapi-typescript-helpers/dist/
andpackages/openapi-typescript-helpers/dist/cjs/
contain anindex.ts
and anindex.d.ts
file.Checklist
docs/
updated (if necessary)pnpm run update:examples
run (only applicable for openapi-typescript)